fix(llm): recognize raw <tool_call> wrappers as function calls - #4608
fix(llm): recognize raw <tool_call> wrappers as function calls#4608sambai-dev wants to merge 2 commits into
Conversation
|
👋 This PR needs a couple of things fixed before OpenHands can review it:
Push an update once this is addressed and this check re-runs automatically. This is an automated check - no AI was used to generate this comment. |
|
🚦 CI is currently failing on this PR's latest commit. Please fix the failing checks before OpenHands reviews it - this is re-checked automatically once you push a new commit. (A maintainer can also request This is an automated check - no AI was used to generate this comment. |
|
Follow-up Validation on the current head:
The current description check confirms the HUMAN/template requirements pass; its only remaining error is that #4540 still lacks |
Some models served behind plain OpenAI-compatible endpoints (Kimi K3 via the Synthetic provider) emit their native <tool_call>NAME wrapper instead of our <function=NAME> dialect. The non-native function-call converter only stripped those tags when a well-formed call was already present, so the raw syntax leaked into assistant text and the tool never fired. Rewrite the malformed shape (<tool_call>NAME followed by a <parameter= line) into <function=NAME>. Two guards keep this safe: it only fires when no <function= is present anywhere in the content, and only on the exact malformed shape, so prose merely mentioning the tag is never converted. Fixes OpenHands#4540
8ba7e1c to
4f3b647
Compare
2 participants
HUMAN:
I have reviewed the approach, the diff, and the validation results, and I approve opening this for maintainer review. The change was prepared with AI assistance (Hermes) under my direction; details and full disclosure below.
AGENT:
Why
Models served behind a plain OpenAI-compatible endpoint (observed with Kimi K3 via the Synthetic provider) emit their native
<tool_call>NAME ...wrapper instead of this SDK's<function=NAME>dialect. The non-native function-call converter only strips those wrapper tags when a well-formed<function=call is already present, so the raw syntax leaks into the assistant text and the tool never fires. Reproduction and raw model output are in #4540.Summary
_TOOL_CALL_WRAPPER_PATTERNto_preprocess_model_output()so a raw<tool_call>NAMEfollowed by a<parameter=...>line is rewritten into the canonical<function=NAME>form, and the first</tool_call>becomes</function>.<function=exists anywhere in the content, and only on the exact malformed shape, so prose merely mentioning the tag is never converted. Truncated calls are completed by the existing_fix_stopwordcloser-append path.Issue Number
Fixes #4540
How to Test
From a local checkout of this branch:
Expected: 33 passed, including the three new
tool_call_wrappertests. On unmodified main the two normalization tests fail withStopIterationbecause no tool call is produced.Full module:
uv run pytest tests/sdk/llm/ -qgives 981 passed and 1 failure (test_llm_profile_store.py::test_default_provider_store_is_sibling_of_base_dir) which fails identically on unmodified main on this Windows machine (verified viagit stashre-run); pre-existing environment issue, untouched here. Lint and types:uv run ruff format && uv run ruff checkanduv run pyrighton both changed files are clean.Video/Screenshots
Not applicable: library-level change with no UI. Command output evidence is included under How to Test.
Risk
This changes tool-call parsing behavior, so per the repository review guide it belongs in eval-risk territory: a COMMENT review with lightweight evals before approval seems right rather than a fast approve. The guards keep the blast radius narrow, and the new tests cover both directions. No public API or persisted-event shapes change.
AI assistance
AI-assisted: investigation and implementation were prepared with Hermes; all reported checks were executed against this branch. The approach follows the analysis posted in #4540 by @VascoSch92.